Periodic task querying is a separate method #883
Merged
+17
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maciej-gol
force-pushed
the
mgol/schedules-separate-method
branch
from
908d76b to
d51be79
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #883 +/- ##
==========================================
+ Coverage 88.07% 88.11% +0.04%
==========================================
Files 32 32
Lines 1006 1010 +4
Branches 104 104
==========================================
+ Hits 886 890 +4
Misses 101 101
Partials 19 19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
auvipy
requested changes
May 5, 2025
Member
auvipy
left a comment
auvipy
left a comment
There was a problem hiding this comment.
thanks for the patch. can you please also add some sort of failing test for this change?
Contributor
Author
|
What do you mean a failing test? :D Its a refactor so there should be no
behavioral changes. I can add tests for each method separately, though.
pon., 5 maj 2025, 10:18 użytkownik Asif Saif Uddin ***@***.***>
napisał:
… ***@***.**** requested changes on this pull request.
thanks for the patch. can you please also add some sort of failing test
for this change?
—
Reply to this email directly, view it on GitHub
<#883 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOGQTONUJM4QREWOOBFBDD244NG7AVCNFSM6AAAAAB4MU6XKSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQMJUGEYTEMJRGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Member
|
Some Additional tests would be nice |
Member
|
@alirafiei75 can you take a look into it as well please? also we have another PR #886 |
Contributor
|
alirafiei75
approved these changes
May 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recent changes to how periodic tasks are filtered and fetched broke how tenant-schemas-celery fetch all periodic tasks from all schemas.
In order not to copy-paste or duck-type new behavior, the filtering logic has been refactored to a separate method,
enabled_models_qs(). This method does what the original query would do. Next, the queryset is consumed into a list insideenabled_models(), which is also used in the old for-loop.The
enabled_models()method is a good integration point, as we still have access to the unevaluated queryset there, whilst being able to return simple datastructure which is a list.In my case, I would use
enabled_models_qs()to construct the query, and run it against all of the schemas. The result ofenabled_models()will be the union of all the results across all of the schemas.